Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[material-ui][docs] Open Material UI template with CodeSandbox/StackBlitz #43604

Merged
merged 43 commits into from
Sep 11, 2024
Merged

[material-ui][docs] Open Material UI template with CodeSandbox/StackBlitz #43604

merged 43 commits into from
Sep 11, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Sep 4, 2024 •
edited by oliviertassinari
Loading

Iterate on #41469

Preview: https://deploy-preview-43604--material-ui.netlify.app/material-ui/getting-started/templates/sign-in/
Preview: https://deploy-preview-43604--material-ui.netlify.app/material-ui/getting-started/templates/dashboard/

Changes

All of the changes are done for SignIn and Dashboard templates only (once this PR is merged, I will open another PR to apply to the rest of the templates).

  • Fix the dark mode flicker by adopting the CSS variables and refactor the theme usage to theme.vars.
  • Move toggle color mode and theme switching to the docs TemplateFrame.js so that the template code is clean and ready to be used.

TemplateFrame Toolbar

  • Add CodeSandbox and StackBlitz buttons
  • Changed the ToggleColorMode to buttons and move it to center. (it's too tight to have everything on the right side)
  • Changed the theme switcher to icon button with a menu to optimize for small screen
image image

@siriwatknp siriwatknp added proof of concept Studying and/or experimenting with a to be validated approach docs Improvements or additions to the documentation labels Sep 4, 2024
@mui-bot
Copy link

mui-bot commented Sep 4, 2024 •
edited
Loading

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 8c64407

@siriwatknp siriwatknp mentioned this pull request Sep 4, 2024
Open
28 tasks
@siriwatknp siriwatknp changed the title [POC] Open Material UI template via CodeSandbox [material-ui][docs] Open Material UI template with CodeSandbox Sep 5, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 6, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 6, 2024
@siriwatknp siriwatknp changed the title [material-ui][docs] Open Material UI template with CodeSandbox [material-ui][docs] Open Material UI template with CodeSandbox/StackBlitz Sep 6, 2024
siriwatknp
siriwatknp commented Sep 6, 2024
View reviewed changes
.circleci/config.yml
Comment on lines -302 to -307
- run:
name: Update the templates shared themes
command: pnpm template:update-theme
- run:
name: '`pnpm template:update-theme` changes committed?'
command: git add -A && git diff --exit-code --staged
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer need to run the script. The shared theme is the source of truth and it will be bundled with the template when users open CodeSandbox or StackBlitz.

@siriwatknp siriwatknp added scope: docs-infra Specific to the docs-infra product and removed proof of concept Studying and/or experimenting with a to be validated approach labels Sep 6, 2024
@zanivan
Copy link
Contributor

zanivan commented Sep 9, 2024 •
edited
Loading

Hey @siriwatknp I've added a couple tweaks to the Toolbar, and I think it's good to go!

One thing that I think we still need to fix is the template screenshot script. I reckon the toolbar shouldn't be included on those, is it possible? The script is pnpm template:screenshot material-ui

I really like the content of the contributing guide, but its placement on this page feels a bit off. The page is already packed with templates, a Store CTA for premium templates, and Toolpad Core, so adding the guide there might be a bit confusing. How about adding it as a FAQ item like, 'How can I contribute to the free templates?' with a link on the templates page?

@siriwatknp
Copy link
Member Author

Hey @siriwatknp I've added a couple tweaks to the Toolbar, and I think it's good to go!

One thing that I think we still need to fix is the template screenshot script. I reckon the toolbar shouldn't be included on those, is it possible? The script is pnpm template:screenshot material-ui

I really like the content of the contributing guide, but its placement on this page feels a bit off. The page is already packed with templates, a Store CTA for premium templates, and Toolpad Core, so adding the guide there might be a bit confusing. How about adding it as a FAQ item like, 'How can I contribute to the free templates?' with a link on the templates page?

Done!

zanivan
zanivan approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@zanivan zanivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@siriwatknp siriwatknp mentioned this pull request Sep 11, 2024
Merged
@siriwatknp siriwatknp merged commit ff9406a into mui:master Sep 11, 2024
22 checks passed
@siriwatknp siriwatknp mentioned this pull request Sep 11, 2024
Merged
1 task
@aarongarciah
Copy link
Member

aarongarciah commented Sep 11, 2024 •
edited
Loading

some stuff I noticed:

  • The color scheme dropdown trigger is displaying up/down arrows, see screenshot below.
    Screenshot 2024-09-11 at 12 43 03

  • The text in the theme dropdown trigger looks a bit too large, the old one felt more balanced in comparison with the options' text size:

    old new
    Screenshot 2024-09-11 at 12 45 37 Screenshot 2024-09-11 at 12 45 50

  • I think the bar contents being all blue seems to attract more attention because we use blue as a color accent. Not sure it's intentional or not. I think the problem is the bar is too integrated with the rest of the template, we could try make it very obvious that the bar is not part of the template (another background color?), but that's something for the future.

    old
    old

    new
    new

@oliviertassinari oliviertassinari mentioned this pull request Sep 21, 2024
Merged
oliviertassinari
oliviertassinari reviewed Sep 21, 2024
View reviewed changes
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool banner, but it looks like we need to continue to iterate on this:

  1. Doesn't work on mobile:
Screen.Recording.2024-09-22.at.00.02.25.mov
  1. Can't really click on it:
Screen.Recording.2024-09-22.at.00.04.33.mov

@zanivan zanivan mentioned this pull request Sep 26, 2024
Merged
1 task
@zanivan
Copy link
Contributor

zanivan commented Sep 26, 2024

@oliviertassinari sure thing! We're still iterating it on #43708

This was referenced Oct 3, 2024
Open
Open
This was referenced Oct 3, 2024
Open
Open
@Msolmaz4 Msolmaz4 mentioned this pull request Oct 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

@zanivan zanivan zanivan approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@siriwatknp @mui-bot @zanivan @aarongarciah @oliviertassinari